[ZEPPELIN-6543] Handle invokeMethod serialization failure as InterpreterRPCException - #5349
Conversation
|
The exception propagation in One thing: removing } catch (IOException e) {
LOGGER.error("invokeMethod failed", e);
throw new InterpreterRPCException(e.toString());
} |
|
Thanks for the suggestion! I restored the server-side error logging while keeping the InterpreterRPCException propagation, so the stack trace is still available for debugging. |
jongyoul
left a comment
jongyoul
left a comment
There was a problem hiding this comment.
LGTM! Thanks for handling invokeMethod serialization failure as InterpreterRPCException.
|
Merged into master (427bf3a). |
3 participants
What is this PR for?
This PR follows up on ZEPPELIN-6467 / PR #5312.
Following PR #5312,
Resource.serializeObject()propagates serialization failures asIOException. As a result,RemoteInterpreterEventServer.invokeMethod()can receive an exception while re-serializing a remote resource invocation result. The existing handler logged the exception and returned a nullByteBuffer, causing the generated Thrift client to report a missing result instead of preserving the original serialization failure.The Jira issue identified this behavior through code analysis, but the server-side deserialize-and-re-serialize failure path had not yet been reproduced.
This PR adds a regression test using a serializable object that succeeds during the initial serialization and fails during the server-side second serialization. It then changes
invokeMethod()to propagate the failure asInterpreterRPCException, allowing the original error message to reach the caller instead of being converted into an unrelated Thrift missing-result error.The behavior for successfully serialized results is unchanged.
What type of PR is it?
Bug Fix
Todos
InterpreterRPCExceptionWhat is the Jira issue?
[ZEPPELIN-6543]
How should this be tested?
./mvnw test -pl zeppelin-server -Dtest=RemoteInterpreterEventServerTestpasses successfully.Screenshots (if appropriate)
N/A
Questions:
Resource.serializeObject()may returnnullwhen the result is not serializable, but this path is not covered by the regression test in this PR. Should this case also be handled in this PR, or should it be addressed separately?